Skip to content

Conversation

jmgutu
Copy link
Member

@jmgutu jmgutu commented Oct 3, 2025

Description

The force serialize function in [django-debug-toolbar/debug_toolbar/panels/settings.py] was throwing errors for values that are not serializable. The proposed solution returns the serialized value if the value is serializable or returns 'Debug toolbar was unable to parse value'

Fixes #2172

Checklist:

  • I have added the relevant tests for this change.
  • I have added an item to the Pending section of docs/changes.rst.

@jmgutu jmgutu requested a review from tim-schilling October 3, 2025 19:41
@jmgutu jmgutu marked this pull request as draft October 3, 2025 19:55
@jmgutu jmgutu marked this pull request as ready for review October 3, 2025 20:48
Comment on lines +27 to +31
def catch_force_errors(force_function, value):
try:
return force_function(value)
except DjangoUnicodeDecodeError:
return "Debug toolbar was unable to parse value"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few places we use force_str and I suspect we may want to swap all of them out with this new function. Perhaps we should create a sanitize.py file and put this in there with a doc string explaining the interface and what it does. Then use that everywhere we've been using force_str. I'd consider keeping the name as force_str too, but that's just my preference. We'd want to have an explicit test covering the functionality of this new function too.

There are some other sanitizing functions in utils.py that can be moved over too, though that should be a separate commit. Or I can do that.

try:
return force_function(value)
except DjangoUnicodeDecodeError:
return "Debug toolbar was unable to parse value"
Copy link
Member

@tim-schilling tim-schilling Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return "Debug toolbar was unable to parse value"
return "Django Debug Toolbar was unable to parse value."

nitpick: We use the full name in other messaging presented to users.

Comment on lines +16 to +17
* Fixed force_str to catch error and give out default string if value is not
serializable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Fixed force_str to catch error and give out default string if value is not
serializable.
* Updated logic that forces values to strings to render "Django Debug Toolbar was
unable to parse value." when there's a decoding error.

I think we're better off trying to communicate what is going to change from a developer experience here rather than what technically changed. A user may try to figure out why they are getting this message and our documentation may not explain it. Having it here in the change log will help them realize it's intentional and be able to track what the issue/PR is to see why it was changed.

Copy link
Member

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @jmgutu for putting this together quickly. I think there are a few changes we should make to improve the toolbar as a whole and to make the developer experience a bit better. Please let me know if you think differently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Settings panel: DjangoUnicodeDecodeError
2 participants